Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dunno if you want my stuff? #11

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Dunno if you want my stuff? #11

wants to merge 16 commits into from

Conversation

crener
Copy link

@crener crener commented Aug 30, 2021

Hey, I've been playing around with this for a little bit and added/changed a bunch of things. I think that other people would find it useful to get these changes as it adds a bunch of stuff but I dunno if you want to pull them all in?

Changes:

  • Added Circular query shape
  • Added basic job based queries
  • Added demo scene for visually showing queries
  • Changed namespacing, broke up the massive multi-file partial class, some stylistic changes - potentially breaking change
  • Moved files around into sub folders where I think it makes sense
  • Added assembly definitions to better split this code from projects using it
  • Added some basic tests to check query shape collision logic
  • Changed readme to include demo scene preview image
  • Added editor only check to assert when trying to add an entry into a leaf that exceeds the node capacity (great for sanity checking)
  • Got some experimental code that will try to parallelise bulk adding operations (still needs some work though)

Figured I'd drop a line here cause most people wouldn't click through all the Forks to see if there are improvements in them

crener and others added 11 commits July 18, 2021 15:18
@crener
Fixed tests, added assembly definitions, got compiling with up to dat…
c8e7088 
…e pacakges
@crener
Added Circle Range Query Type
b3dbe33
@crener
Allow better native compatability
ffc9874
@crener
Improved burst compatability, alternate way to bulk add entries with …
5f3981f 
…more parallelism
@crener
Major organisation of file structure
d7a8e49
@crener
Fixed some collision calculation logic when using circle and added un…
b15cdf9 
…it tests
@crener
Fixed issues with Circle Range queries, added unit tests for correct …
a1a65c5 
…intersection behaviour
@crener
Created demo scene for debugging results, Fixed inacurate circle resu…
56a4a30 
…lts, added more tests, updated solution to 2021.2
@crener
Found and fixed edge case where center of circle was outside the test…
1f4b0ee 
…ed box
@crener
Added example image to readme
5bd00a2
@crener
fix type in readme
7298aa8
@marijnz
Copy link
Owner

marijnz commented Aug 31, 2021 via email

@crener
Less aggressive range query resize code (less overshoot on resize ope…
98750de 
…rations), Added some validation code for debugging while running,
@crener
Copy link
Author

crener commented Sep 5, 2021

I was playing around with this a little more today and I've found a bug where range queries suddenly return no more results once you have more total entries than the max amount per leaf in the tree even when the entries are spread out across multiple nodes (not actually above the per node limit on any individual node).

I don't know if this is because of my changes or if this is an existing issue, so might want to wait a bit when merging this...

@crener
Copy link
Author

crener commented Sep 8, 2021

So I spent a while trying to figure out what exactly is going on (added a bunch of temp code that exposes the raw structures so I can watch them change) and I've come up with a unit test which illustrates the issue.

Still looking at it when I have time but since figuring out what is going on inside native memory isn't straight forward I don't have a solution for this yet. My current theory is that the shape intersection code doesn't behave as expected when a query shape is a very small part of the QuadTree bounds....

Hope you had a nice Holiday :)

@crener
Copy link
Author

crener commented Sep 11, 2021

Got around to looking at this again and it was more of an issue with the location that I places the node count assert... So I moved the assert so that is tests the nodes correctly which makes the test fail as expected.

but while looking around and actually visualising the data so I could see what was going on I came across an interesting case where I don't know why a QuadElement was sorting in a specific QuadNode.

Untitled
That is MultiDepth2 unit test which does this.

Then MultiDepth3 also seems to do some strange behaviour where the node at depth 2 is populated with a single entry where the depth 1 node for the depth 2 cluster is empty with the leaf element size being at 3... I would have expected that there would be 2 nodes at depth 1 being used... but apparently not?

@marijnz
Copy link
Owner

marijnz commented Sep 18, 2021

Heya, so I'm back :)
Note that I'm not actively working on this project anymore. Otherwise I'd love to debug thing together with you, but I can't find the time for that right now.
Have you continued since the last bug you've found?

@crener
Copy link
Author

crener commented Sep 18, 2021 •
edited
Loading

I've not really done more since I added the failing unit test.

I did try to figure it out but I need to read up on the specifics of how Morton codes work in order to fix it cause it seems like some part of the lookup is failing to set items correctly when doing the recursive add stuff.

There is a unit test though so getting a repo is super easy :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@crener @marijnz